[codex] Structure VCS process boundary errors#3476
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved This PR is a mechanical refactoring that splits a generic error class into multiple specific typed error classes without changing runtime behavior. The original union type is preserved for backward compatibility, and the changes are well-tested. You can customize Macroscope's approvability policy. Learn more. |
Problem
VcsOutputDecodeErrorgrouped stdin writes, process reads, output limits, and missing exit codes behind one tag and a free-formdetailstring. That erased useful byte and stream diagnostics, made the message depend on manually manufactured text, and allowed real I/O causes and structural no-cause failures to share the same loose shape.Change
Replace the single error class with a
Schema.Unionof distinct tagged errors for stdin writes, output reads, output limits, and missing exit codes. The errors now expose safe structured fields such asstream,stdinBytes,maxBytes, andobservedBytes; real stdin/read failures retain their exact underlying cause, while output-limit and missing-code conditions do not manufacture one. Messages derive only from those structured fields and never from a cause or captured output.VcsProcessconstructs the errors directly at the mapping boundary, and the exhaustive GitLab CLI mapper recognizes the new tags without changing its existing fallback behavior. The focused backend test verifies exact-cause identity, safe messages, and the absence of a synthetic cause for a missing exit code.Validation
vp test run apps/server/src/vcs/VcsProcess.test.ts apps/server/src/sourceControl/GitLabCli.test.ts(18 tests)vp check(passes; existing unrelated warnings remain)vp run typecheckNote
Medium Risk
Touches shared
@t3tools/contractserror shapes and the coreVcsProcessboundary; callers matching onVcsErrortags must handle four tags instead of one, though the union alias preserves grouping.Overview
Replaces the single
VcsOutputDecodeError(free-formdetail) with four distinct tagged errors:VcsProcessStdinWriteError,VcsProcessOutputReadError,VcsProcessOutputLimitError, andVcsProcessMissingExitCodeError. Each exposes structured fields (stdinBytes,stream,maxBytes,observedBytes) and messages derived from those fields only; real I/O failures keep their underlyingcause, while limit and missing-exit-code cases do not invent one.VcsProcessmapsProcessRunnerfailures directly to these types instead of factory helpers on the old class.VcsErrorlists the four tags individually;VcsOutputDecodeErrorremains as aSchema.Unionalias for backward grouping. GitLab CLI’s exhaustivefromVcsErrormatcher adds cases for each new tag, all falling through toGitLabCliCommandErrorlike before.Tests add a mocked
ProcessRunnerpath to assert exact cause identity, safe messages, and no syntheticcauseon missing exit code.Reviewed by Cursor Bugbot for commit a6747ab. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure VCS process boundary errors into granular error types
VcsOutputDecodeErrorclass invcs.tswith four dedicated error classes:VcsProcessStdinWriteError,VcsProcessOutputReadError,VcsProcessOutputLimitError, andVcsProcessMissingExitCodeError, each with structured fields and specific tags.VcsProcess.tsto emit these granular types instead of the oldVcsOutputDecodeErrorfactory methods, preserving underlying causes for stdin/read failures.GitLabCli.tsto map each new error tag to aGitLabCliCommandErrorwith the original cause preserved.VcsOutputDecodeErroris retained as a union type alias over the four new classes for backward compatibility with existing consumers.VcsErrorconsumers will now see distinct tags per boundary error type rather than a single genericVcsOutputDecodeErrortag.Macroscope summarized a6747ab.